Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

apps/web/src/tests/browser/shared/abstractions/components/schema-editor.component.js (1)

189-208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider consolidating the near-duplicate params-dialog helpers.

editRowParamsWithDialog and editRowEnumParamsWithDialog differ only in which paramsEditor setter is invoked per entry. A single helper taking a setter callback would avoid future drift between the two.

♻️ Proposed consolidation

-  async editRowParamsWithDialog(index, valuesByName) {
-    await this.ensureSchemaMode();
-    await this.dismissOpenHelpTooltips();
-    await this.row(index).locator('[data-action="edit-params"]').click();
-    for (const [name, value] of Object.entries(valuesByName || {})) {
-      await this.paramsEditor.setValue(name, value);
-    }
-    await this.paramsEditor.apply();
-  }
-
-  async editRowEnumParamsWithDialog(index, valuesByName) {
-    await this.ensureSchemaMode();
-    await this.dismissOpenHelpTooltips();
-    await this.row(index).locator('[data-action="edit-params"]').click();
-    for (const [name, value] of Object.entries(valuesByName || {})) {
-      await this.paramsEditor.selectEnumValue(name, value);
-    }
-    await this.paramsEditor.apply();
-  }
+  async editRowParamsWithDialog(index, valuesByName, { setter = 'setValue' } = {}) {
+    await this.ensureSchemaMode();
+    await this.dismissOpenHelpTooltips();
+    await this.row(index).locator('[data-action="edit-params"]').click();
+    for (const [name, value] of Object.entries(valuesByName || {})) {
+      await this.paramsEditor[setter](name, value);
+    }
+    await this.paramsEditor.apply();
+  }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/web/src/tests/browser/shared/abstractions/components/schema-editor.component.js`
around lines 189 - 208, The two params-dialog helpers are near-duplicates, so
consolidate them into a single shared flow in schema-editor.component.js.
Refactor editRowParamsWithDialog and editRowEnumParamsWithDialog to use one
internal helper that handles ensureSchemaMode, dismissOpenHelpTooltips,
row(...).locator('[data-action="edit-params"]').click(), iteration over
valuesByName, and paramsEditor.apply(), while accepting a setter callback to
choose between paramsEditor.setValue and paramsEditor.selectEnumValue. Keep the
existing public method names as thin wrappers if needed.

packages/core/js/keywords/domain/word/adverb-keyword-definition.js (1)

3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting WORD_SELECTION_STRATEGY_TYPE into a shared constant.

This same array is now duplicated verbatim across adverb, conjunction, noun, preposition, sample, and verb keyword definitions (and likely adjective/interjection). A single source of truth would prevent future drift if the allowed strategies ever change.

♻️ Suggested consolidation

+// packages/core/js/keywords/domain/word/word-selection-strategy.js
+export const WORD_SELECTION_STRATEGY_TYPE = ['fail', 'closest', 'shortest', 'longest', 'any-length'];

Then import it in each word/*-keyword-definition.js file instead of redeclaring.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/js/keywords/domain/word/adverb-keyword-definition.js` at line
3, Extract the duplicated WORD_SELECTION_STRATEGY_TYPE array into a shared
constant and import it from each word keyword definition instead of redeclaring
it. Update adverbKeywordDefinition and the other word/*-keyword-definition.js
modules that currently define the same array so they all reference the shared
source of truth, keeping the existing symbol name available where needed.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/core/js/data_generation/utils/enumParser.js (1)

106-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated "unquoted empty field" check across three call sites.

The predicate field.value.length === 0 && !field.quoted (or its negation via .every) is reimplemented independently in isImplicitCsvEnumRuleSpec (Line 114), the implicit-CSV branch of parseEnumRuleSpec (Line 184), and parseCsvEnumValues (Line 418). Extracting a shared helper (e.g. hasUnquotedEmptyField(fields)) would keep these three call sites in sync as the empty-value rules evolve.

♻️ Suggested consolidation

+  static hasUnquotedEmptyField(fields) {
+    return fields.some((field) => field.value.length === 0 && !field.quoted);
+  }
+
   static isImplicitCsvEnumRuleSpec(ruleSpec) {
     ...
     try {
       const fields = this.parseCsvLiteralFields(spec);
-      return fields.length >= 2 && fields.every((field) => field.value.length > 0 || field.quoted);
+      return fields.length >= 2 && !this.hasUnquotedEmptyField(fields);
     } catch {
       return false;
     }
   }

Also applies to: 180-192, 398-422

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/js/data_generation/utils/enumParser.js` around lines 106 - 118,
The empty unquoted CSV field check is duplicated across
isImplicitCsvEnumRuleSpec, the implicit-CSV branch in parseEnumRuleSpec, and
parseCsvEnumValues. Extract a shared helper for this rule (for example, a
predicate over the parsed fields) and update all three call sites to use it so
the empty-value behavior stays consistent in EnumParser.